Skip to content

Conversation

@theshadowco
Copy link
Member

@theshadowco theshadowco commented Jan 16, 2026

Описание

  1. исправлены ошибки при чтении ролей (зависимости)
  2. исправлен алгоритм вычисления областей в комментарии

Связанные задачи

Closes #3770
Closes #3772

Чеклист

Общие

  • Ветка PR обновлена из develop
  • Отладочные, закомментированные и прочие, не имеющие смысла участки кода удалены
  • Изменения покрыты тестами
  • Обязательные действия перед коммитом выполнены (запускал команду gradlew precommit)

Для диагностик

  • Описание диагностики заполнено для обоих языков (присутствуют файлы для обоих языков, для русского заполнено все подробно, перевод на английский можно опустить)

Дополнительно

Summary by CodeRabbit

  • Dependencies

    • Updated build script to use newer parser and module package versions.
  • Improvements

    • Adjusted preprocessor token grouping so directive tokens are combined for more consistent highlighting.
    • Improved document-token line mapping to better align emitted tokens with source lines.
  • Tests

    • Tests rewritten to use explicit expected-token helpers and updated expectations to match tokenization changes.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 16, 2026

📝 Walkthrough

Walkthrough

Bumps two 1c-syntax dependencies, changes preprocessor semantic-token grouping to merge leading # with the first preproc keyword, and modernizes many semantic-token tests to use ExpectedToken-based helper assertions and updated expectations.

Changes

Cohort / File(s) Summary
Dependency updates
build.gradle.kts
Bumped io.github.1c-syntax:bsl-parser 0.30.0-rc.2 → 0.30.0-rc.5 and io.github.1c-syntax:mdclasses 0.17.3 → 0.17.4 (exclusions unchanged).
Preprocessor token grouping
src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/PreprocessorSemanticTokensSupplier.java
Reworked addOtherPreprocs to combine a leading HASH and the first following PREPROC_ into one Macro token range; remaining PREPROC_ tokens stay as individual Macro tokens. Review token range logic and tests.
Per-line BSL doc lookup change
src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/BslDocSemanticTokensSupplier.java
Lookup now uses absolute fileLine key instead of iteration index lineIdx when collecting per-line elements; may affect which elements produce tokens for some lines.
Hyperlink assertion tweaks
src/test/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/description/MethodDescriptionTest.java
Tests compare hyperlink string via hyperlink().link() instead of comparing Hyperlink objects directly.
Semantic-tokens tests — assertion modernization
src/test/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/...
Multiple tests converted to use ExpectedToken helpers (helper.assertContainsTokens, helper.assertTokensMatch), removed legend-dependent assertions and static assertThat. Files include: BslDocSemanticTokensSupplierTest.java, CommentSemanticTokensSupplierTest.java, LexicalSemanticTokensSupplierTest.java, PreprocessorSemanticTokensSupplierTest.java, StringSemanticTokensSupplierTest.java, SymbolsSemanticTokensSupplierTest.java.
Provider test adjustments
src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProviderTest.java
Added two leading lines in methodDescriptionComments sample shifting expected token line indices; updated expected tokens for preprocessor macro merging and documentation token reindexing.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • #3764 — also updates mdclasses version in build.gradle.kts.
  • #3651 — also updates bsl-parser version in build.gradle.kts.
  • #3659 — related changes to preprocessor semantic-token handling (merging/grouping of HASH/PREPROC tokens).

Suggested labels

hacktoberfest-accepted

Suggested reviewers

  • nixel2007

Poem

🐰 I hopped through lines of code today,

Merged a hash and helped tests play,
Bumped a lib, aligned the view,
Tokens sing a tidier tune anew,
A tiny hop — the build feels gay.

🚥 Pre-merge checks | ✅ 1 | ❌ 4
❌ Failed checks (3 warnings, 1 inconclusive)
Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR changes do not appear to address the XML parsing bugs documented in issues #3770 and #3772; instead, the changes focus on semantic tokenization, test refactoring, and dependency updates. Verify that bug fixes for RoleDataConverter's reader.moveUp() issue and consecutive XML tag handling (#3770, #3772) are included. Current changes appear unrelated to role/rights parsing fixes.
Out of Scope Changes check ⚠️ Warning Multiple changes appear unrelated to the stated objectives: dependency version updates (bsl-parser, mdclasses), widespread test refactoring for semantic token assertions, and BslDocSemanticTokensSupplier logic changes unrelated to role parsing. Remove or justify out-of-scope changes: dependency upgrades, test assertion rewrites, and semantic token supplier refactoring that don't address role/rights XML parsing bugs.
Docstring Coverage ⚠️ Warning Docstring coverage is 10.71% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive PR title 'Исправление ошибок в ролях и описании' is vague and generic, using non-specific language that doesn't clearly describe the technical implementation details of the changeset. Clarify the title to specify the core technical fix, e.g. mentioning XML parsing correction, Reader.moveUp() fix, or semantic token restructuring rather than relying on vague 'fixes' terminology.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings


📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f9ea57d and 89bb49a.

📒 Files selected for processing (2)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/BslDocSemanticTokensSupplier.java
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProviderTest.java
🧰 Additional context used
📓 Path-based instructions (3)
src/main/java/**/*.java

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

src/main/java/**/*.java: Follow the Style Guide for code formatting and conventions
Use Lombok annotations to reduce boilerplate code and enable annotation processing in IDE
Write JavaDoc for public APIs and include comments for complex logic
Use meaningful, descriptive names for classes and methods following Java naming conventions
Optimize imports before committing; DO NOT optimize imports across the entire project unless specifically working on that task

Files:

  • src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/BslDocSemanticTokensSupplier.java
src/**/*.java

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use Java 17 for language and follow Java naming conventions

Files:

  • src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/BslDocSemanticTokensSupplier.java
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProviderTest.java
src/test/java/**/*Test.java

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Maintain or improve test coverage and use appropriate test frameworks (JUnit, AssertJ, Mockito)

Files:

  • src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProviderTest.java
🧠 Learnings (2)
📓 Common learnings
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 0
File: :0-0
Timestamp: 2025-12-28T11:15:32.357Z
Learning: In PR `#3706` about semantic token optimization, I incorrectly stated that the algorithm in findSuffixMatchWithOffset correctly handles inline text insertions (text insertion without line breaks). The actual bug is that when lineOffset=0, tokens after the insertion point have the same deltaLine but different deltaStart, and the old algorithm failed to match them because it checked deltaStart equality. PR `#3707` fixes this by allowing deltaStart to differ for the boundary token when lineOffset==0.
📚 Learning: 2025-12-28T11:15:32.357Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 0
File: :0-0
Timestamp: 2025-12-28T11:15:32.357Z
Learning: In PR `#3706` about semantic token optimization, I incorrectly stated that the algorithm in findSuffixMatchWithOffset correctly handles inline text insertions (text insertion without line breaks). The actual bug is that when lineOffset=0, tokens after the insertion point have the same deltaLine but different deltaStart, and the old algorithm failed to match them because it checked deltaStart equality. PR `#3707` fixes this by allowing deltaStart to differ for the boundary token when lineOffset==0.

Applied to files:

  • src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/BslDocSemanticTokensSupplier.java
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProviderTest.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
  • GitHub Check: Analyse
  • GitHub Check: build
  • GitHub Check: Benchmark
  • GitHub Check: build (17, ubuntu-latest)
  • GitHub Check: build (25, ubuntu-latest)
  • GitHub Check: build (25, macOS-latest)
  • GitHub Check: build (21, windows-latest)
  • GitHub Check: build (21, macOS-latest)
  • GitHub Check: build (25, windows-latest)
  • GitHub Check: build (21, ubuntu-latest)
  • GitHub Check: build (17, windows-latest)
  • GitHub Check: build (17, macOS-latest)
🔇 Additional comments (2)
src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProviderTest.java (2)

349-360: LGTM!

The updated expected tokens correctly reflect the merged preprocessor directive behavior where # and the following keyword are combined into a single Macro token. Token lengths and line positions are accurate.


388-443: Test expectations look correct.

The expected tokens properly reflect the updated BSL sample with two leading lines. Line indices in ExpectedToken entries match the actual source positions:

  • Line 0: plain comment (no Documentation modifier) ✓
  • Line 2-4: documentation block with proper keyword/parameter/type highlighting ✓
  • Line 5: procedure signature ✓
  • Line 6: body comment (no Documentation modifier) ✓
  • Line 7: end procedure ✓

Minor documentation nit: inline comments at lines 403-405 reference "Line 3", "Line 4" but the corresponding ExpectedTokens correctly use lines 2, 3, 4. This doesn't affect test correctness.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@nixel2007 nixel2007 enabled auto-merge January 16, 2026 08:37
@github-actions
Copy link
Contributor

github-actions bot commented Jan 16, 2026

Test Results

 2 907 files  ±0   2 907 suites  ±0   1h 1m 8s ⏱️ + 2m 19s
 1 227 tests ±0   1 226 ✅  - 1  0 💤 ±0  1 ❌ +1 
11 043 runs  ±0  11 034 ✅  - 9  0 💤 ±0  9 ❌ +9 

For more details on these failures, see this check.

Results for commit f9ea57d. ± Comparison against base commit f4137ab.

♻️ This comment has been updated with latest results.

Copilot AI and others added 4 commits January 16, 2026 10:08
…cSemanticTokensSupplierTest

Co-authored-by: nixel2007 <1132840+nixel2007@users.noreply.github.com>
…emantic tokens supplier tests

- BslDocSemanticTokensSupplierTest: use assertTokensMatch/assertContainsTokens with explicit tokens
- CommentSemanticTokensSupplierTest: use assertTokensMatch for multiline test
- LexicalSemanticTokensSupplierTest: use assertContainsTokens with explicit tokens
- PreprocessorSemanticTokensSupplierTest: use assertTokensMatch, fix #Если to be single token
- StringSemanticTokensSupplierTest: use assertTokensMatch instead of hasSize + assertContainsTokens
- SymbolsSemanticTokensSupplierTest: use assertContainsTokens with explicit tokens

Also fix PreprocessorSemanticTokensSupplier to combine # and keyword into single token for #Если, #КонецЕсли etc.

Co-authored-by: nixel2007 <1132840+nixel2007@users.noreply.github.com>
Co-authored-by: nixel2007 <1132840+nixel2007@users.noreply.github.com>
@nixel2007
Copy link
Member

/buildJar

@github-actions
Copy link
Contributor

✅ Собраны JAR-файлы для этого PR по команде /buildJar.

Артефакт: 5152950316

Файлы внутри:

  • bsl-language-server-feature-fixes260116-c82f929-exec.jar

@nixel2007
Copy link
Member

/buildJar

@github-actions
Copy link
Contributor

✅ Собраны JAR-файлы для этого PR по команде /buildJar.

Артефакт: 5152979827

Файлы внутри:

  • bsl-language-server-feature-fixes260116-2af58b9-exec.jar

2. исправлен алгоритм вычисления областей в комментарии
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In
`@src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProviderTest.java`:
- Around line 424-443: The expected token line indices are off by one: the
comment tokens for " - ", "Число" and " - описание" belong to the same source
line containing "//  Парам - Число - описание" and should use line 4 instead of
5, and the subsequent procedure/body/end tokens (the ExpectedToken entries
referencing "Процедура", "ДокТест", "(", "Парам", ")", the body comment "//
обычный комментарий", and "КонецПроцедуры") must all be shifted up one line as
well (decrement their line numbers by 1) so the ExpectedToken entries match the
actual source lines.
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c82f929 and 74007bf.

📒 Files selected for processing (10)
  • build.gradle.kts
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/PreprocessorSemanticTokensSupplier.java
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/description/MethodDescriptionTest.java
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProviderTest.java
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/BslDocSemanticTokensSupplierTest.java
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/CommentSemanticTokensSupplierTest.java
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/LexicalSemanticTokensSupplierTest.java
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/PreprocessorSemanticTokensSupplierTest.java
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/StringSemanticTokensSupplierTest.java
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/SymbolsSemanticTokensSupplierTest.java
🚧 Files skipped from review as they are similar to previous changes (2)
  • build.gradle.kts
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/description/MethodDescriptionTest.java
🧰 Additional context used
📓 Path-based instructions (3)
src/main/java/**/*.java

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

src/main/java/**/*.java: Follow the Style Guide for code formatting and conventions
Use Lombok annotations to reduce boilerplate code and enable annotation processing in IDE
Write JavaDoc for public APIs and include comments for complex logic
Use meaningful, descriptive names for classes and methods following Java naming conventions
Optimize imports before committing; DO NOT optimize imports across the entire project unless specifically working on that task

Files:

  • src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/PreprocessorSemanticTokensSupplier.java
src/**/*.java

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use Java 17 for language and follow Java naming conventions

Files:

  • src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/PreprocessorSemanticTokensSupplier.java
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/StringSemanticTokensSupplierTest.java
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/CommentSemanticTokensSupplierTest.java
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/SymbolsSemanticTokensSupplierTest.java
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/BslDocSemanticTokensSupplierTest.java
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/PreprocessorSemanticTokensSupplierTest.java
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/LexicalSemanticTokensSupplierTest.java
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProviderTest.java
src/test/java/**/*Test.java

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Maintain or improve test coverage and use appropriate test frameworks (JUnit, AssertJ, Mockito)

Files:

  • src/test/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/StringSemanticTokensSupplierTest.java
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/CommentSemanticTokensSupplierTest.java
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/SymbolsSemanticTokensSupplierTest.java
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/BslDocSemanticTokensSupplierTest.java
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/PreprocessorSemanticTokensSupplierTest.java
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/LexicalSemanticTokensSupplierTest.java
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProviderTest.java
🧠 Learnings (2)
📓 Common learnings
Learnt from: theshadowco
Repo: 1c-syntax/bsl-language-server PR: 3610
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/folding/QueryPackageFoldingRangeSupplier.java:26-26
Timestamp: 2025-11-19T09:02:03.154Z
Learning: В проекте bsl-language-server используется кастомная версия ANTLR4 от 1c-syntax (io.github.1c-syntax:antlr4), которая включает базовый класс org.antlr.v4.runtime.Tokenizer с методами getAst() и getTokens(). SDBLTokenizer и BSLTokenizer наследуются от этого базового класса.
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 0
File: :0-0
Timestamp: 2025-12-28T11:15:32.357Z
Learning: In PR `#3706` about semantic token optimization, I incorrectly stated that the algorithm in findSuffixMatchWithOffset correctly handles inline text insertions (text insertion without line breaks). The actual bug is that when lineOffset=0, tokens after the insertion point have the same deltaLine but different deltaStart, and the old algorithm failed to match them because it checked deltaStart equality. PR `#3707` fixes this by allowing deltaStart to differ for the boundary token when lineOffset==0.
📚 Learning: 2025-12-28T11:15:32.357Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 0
File: :0-0
Timestamp: 2025-12-28T11:15:32.357Z
Learning: In PR `#3706` about semantic token optimization, I incorrectly stated that the algorithm in findSuffixMatchWithOffset correctly handles inline text insertions (text insertion without line breaks). The actual bug is that when lineOffset=0, tokens after the insertion point have the same deltaLine but different deltaStart, and the old algorithm failed to match them because it checked deltaStart equality. PR `#3707` fixes this by allowing deltaStart to differ for the boundary token when lineOffset==0.

Applied to files:

  • src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProviderTest.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
  • GitHub Check: build
  • GitHub Check: build (25, windows-latest)
  • GitHub Check: build (25, macOS-latest)
  • GitHub Check: build (25, ubuntu-latest)
  • GitHub Check: build (17, ubuntu-latest)
  • GitHub Check: build (21, macOS-latest)
  • GitHub Check: build (17, windows-latest)
  • GitHub Check: build (21, windows-latest)
  • GitHub Check: build (21, ubuntu-latest)
  • GitHub Check: build (17, macOS-latest)
  • GitHub Check: Analyse
🔇 Additional comments (10)
src/test/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/LexicalSemanticTokensSupplierTest.java (2)

66-83: Clear test intent and correct expectations.

The updated assertions correctly validate that lexical tokens include keywords/operators while excluding regular string tokens.


160-176: Accurate separation of responsibilities for query strings.

The test now correctly asserts that query string content is handled elsewhere while keeping keyword/operator checks here.

src/test/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/CommentSemanticTokensSupplierTest.java (1)

135-147: Helper-based assertions look solid.

Clear expected-token coverage for both multiline-disabled and multiline-enabled paths.

src/test/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/PreprocessorSemanticTokensSupplierTest.java (1)

101-108: Explicit token expectations match the new grouping behavior.

Good alignment with the updated preprocessor tokenization logic.

src/test/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/StringSemanticTokensSupplierTest.java (1)

62-85: Helper-based assertions are consistent and clear.

Good migration to assertTokensMatch with explicit expectations.

Also applies to: 399-401

src/test/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/SymbolsSemanticTokensSupplierTest.java (1)

60-104: Nice upgrade to explicit ExpectedToken checks.

Improves clarity and consistency across symbol token tests.

Also applies to: 119-123

src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/PreprocessorSemanticTokensSupplier.java (1)

101-133: The code correctly handles preprocessor directives without duplication. The use and native directives are separate top-level grammar rules, not children of preprocessor, so they are never processed by addOtherPreprocs(). No guard is needed.

src/test/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/BslDocSemanticTokensSupplierTest.java (3)

195-202: Clear, line-precise type expectations.

Explicit ExpectedToken assertions for multiple types across lines are precise and improve coverage.


222-227: Good explicit checks for multiple return types.

The ExpectedToken entries make the test intent and positions unambiguous.


243-265: Multiline vs single-line coverage is solid.

Covering both modes with explicit ExpectedToken sets strengthens regression safety.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

Copilot AI and others added 2 commits January 16, 2026 13:21
…ct combined tokens (#3780)

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: nixel2007 <1132840+nixel2007@users.noreply.github.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/BslDocSemanticTokensSupplier.java (1)

209-225: Fix coordinate system inconsistency in addBslDocTokensWithMultilineSupport.

The multiline support method at lines 161 and 167 uses lineIdx (relative index into the lines array) for lookups, but elementsByLine is keyed by file-absolute line numbers (element.line()). In contrast, addBslDocTokensPerLine at line 215 correctly uses fileLine (fileStartLine + lineIdx).

When fileStartLine != 0, the lookups at lines 161 and 167 will fail to find elements that are stored with file-absolute coordinates, causing tokens to be missed.

Change both occurrences to use file-absolute line numbers:

var lineElements = elementsByLine.getOrDefault(fileLine, List.of());  // line 161
var nextLineElements = elementsByLine.getOrDefault(fileLine, List.of());  // line 167
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f9ea57d and 89bb49a.

📒 Files selected for processing (2)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/BslDocSemanticTokensSupplier.java
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProviderTest.java
🧰 Additional context used
📓 Path-based instructions (3)
src/main/java/**/*.java

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

src/main/java/**/*.java: Follow the Style Guide for code formatting and conventions
Use Lombok annotations to reduce boilerplate code and enable annotation processing in IDE
Write JavaDoc for public APIs and include comments for complex logic
Use meaningful, descriptive names for classes and methods following Java naming conventions
Optimize imports before committing; DO NOT optimize imports across the entire project unless specifically working on that task

Files:

  • src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/BslDocSemanticTokensSupplier.java
src/**/*.java

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use Java 17 for language and follow Java naming conventions

Files:

  • src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/BslDocSemanticTokensSupplier.java
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProviderTest.java
src/test/java/**/*Test.java

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Maintain or improve test coverage and use appropriate test frameworks (JUnit, AssertJ, Mockito)

Files:

  • src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProviderTest.java
🧠 Learnings (2)
📓 Common learnings
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 0
File: :0-0
Timestamp: 2025-12-28T11:15:32.357Z
Learning: In PR `#3706` about semantic token optimization, I incorrectly stated that the algorithm in findSuffixMatchWithOffset correctly handles inline text insertions (text insertion without line breaks). The actual bug is that when lineOffset=0, tokens after the insertion point have the same deltaLine but different deltaStart, and the old algorithm failed to match them because it checked deltaStart equality. PR `#3707` fixes this by allowing deltaStart to differ for the boundary token when lineOffset==0.
📚 Learning: 2025-12-28T11:15:32.357Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 0
File: :0-0
Timestamp: 2025-12-28T11:15:32.357Z
Learning: In PR `#3706` about semantic token optimization, I incorrectly stated that the algorithm in findSuffixMatchWithOffset correctly handles inline text insertions (text insertion without line breaks). The actual bug is that when lineOffset=0, tokens after the insertion point have the same deltaLine but different deltaStart, and the old algorithm failed to match them because it checked deltaStart equality. PR `#3707` fixes this by allowing deltaStart to differ for the boundary token when lineOffset==0.

Applied to files:

  • src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/BslDocSemanticTokensSupplier.java
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProviderTest.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
  • GitHub Check: Analyse
  • GitHub Check: build
  • GitHub Check: Benchmark
  • GitHub Check: build (17, ubuntu-latest)
  • GitHub Check: build (25, ubuntu-latest)
  • GitHub Check: build (25, macOS-latest)
  • GitHub Check: build (21, windows-latest)
  • GitHub Check: build (21, macOS-latest)
  • GitHub Check: build (25, windows-latest)
  • GitHub Check: build (21, ubuntu-latest)
  • GitHub Check: build (17, windows-latest)
  • GitHub Check: build (17, macOS-latest)
🔇 Additional comments (2)
src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProviderTest.java (2)

349-360: LGTM!

The updated expected tokens correctly reflect the merged preprocessor directive behavior where # and the following keyword are combined into a single Macro token. Token lengths and line positions are accurate.


388-443: Test expectations look correct.

The expected tokens properly reflect the updated BSL sample with two leading lines. Line indices in ExpectedToken entries match the actual source positions:

  • Line 0: plain comment (no Documentation modifier) ✓
  • Line 2-4: documentation block with proper keyword/parameter/type highlighting ✓
  • Line 5: procedure signature ✓
  • Line 6: body comment (no Documentation modifier) ✓
  • Line 7: end procedure ✓

Minor documentation nit: inline comments at lines 403-405 reference "Line 3", "Line 4" but the corresponding ExpectedTokens correctly use lines 2, 3, 4. This doesn't affect test correctness.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
B Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@nixel2007 nixel2007 merged commit cbf9bc9 into develop Jan 16, 2026
38 of 39 checks passed
@nixel2007 nixel2007 deleted the feature/fixes260116 branch January 16, 2026 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants